In `@src/app/debug/bookings/page.tsx`:
- Around line 322-334: The onChange handler for DateInput uses
v.toISOString().slice(0,10) when v is a Date which converts to UTC and causes
off-by-one days in UTC+ timezones; update the branch handling v as a Date to
build the local date instead (use v.getFullYear(), v.getMonth(), v.getDate() or
construct new Date(v.getFullYear(), v.getMonth(), v.getDate())) and then call
setSelectedDay with that local-date value; keep the string branch parsing logic
for string inputs unchanged and ensure you still null-check v and call
setSelectedDay(null) when v is null (locations: the onChange handler, variable
v, and setSelectedDay).

In `@src/lib/datetime.ts`:
- Around line 3-14: Update the docstring in src/lib/datetime.ts for the rounding
function (the top comment block describing "Rounds a date up to the nearest time
slot increment") to correct the example showing 14:45 → 15:00; change it to
14:45 → 14:45 so the example accurately reflects that inputs already on the
increment boundary are unchanged (e.g., 14:07 → 14:15, 14:15 → 14:15, 14:16 →
14:30, 14:45 → 14:45).

In `@src/lib/google-maps.ts`:
- Around line 34-42: The JSDoc for the Google Maps travel-time function is
inaccurate: only the missing-API-key check actually throws, while HTTP/API
errors are caught and the function returns null; update the docblock for the
function (the "Calculates travel time between two addresses..." comment) to
remove or narrow the `@throws` description to only the missing API key case
(mention the specific missing-key check) and clarify in `@returns` that API
request failures result in a null return rather than an exception; reference the
function name/description in the file so you update the correct comment.

In `@src/server/api/routers/bookings.ts`:
- Around line 514-528: The current guard only calls validateDriverForSlot when
updatesToApply.driverId is present, so changes to startTime/endTime/addresses on
a booking with an existing driver skip re-validation; change the condition to
also run validation when there is an existing.driverId and any of
startTime/endTime/pickupAddress/destinationAddress are being changed. Inside
that block use driverId = updatesToApply.driverId ?? existing.driverId and
compute proposedStart/proposedEnd/proposedPickup/proposedDest the same way, then
await validateDriverForSlot(ctx, { driverId, startTime: proposedStart, endTime:
proposedEnd, pickupAddress: proposedPickup, destinationAddress: proposedDest,
excludeBookingId: id }); ensure the condition references
updatesToApply.startTime, updatesToApply.endTime, updatesToApply.pickupAddress,
and updatesToApply.destinationAddress and existing.driverId.
- Around line 222-234: listDrivers currently returns driver emails to any
authenticated caller; restrict access by checking the caller's role before
selecting the email column. In the listDrivers handler (protectedProcedure ->
listDrivers) either (A) enforce admin-only access by using your admin middleware
or checking ctx.session.user.role === "admin" and aborting with an unauthorized
error if not admin, or (B) keep the route public to authenticated users but
remove the email from the select for non-admins (select only id and name when
ctx.session.user.role !== "admin"). Update the select projection (the current
select of user.id, user.name, user.email) and the access check in listDrivers
accordingly so emails are only returned to admins.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/app/debug/bookings/page.tsx`:
- Around line 53-65: The function bookingOverlapsDay mixes local-time day
boundaries with UTC booking timestamps; update bookingOverlapsDay to compute
dayStart/dayEnd in UTC (e.g., construct dayStart via
Date.UTC(day.getUTCFullYear(), day.getUTCMonth(), day.getUTCDate()) or call
setUTCHours(0,0,0,0) and dayEnd via setUTCHours(23,59,59,999)) so that the day
boundaries and the parsed booking times (new Date(booking.startTime)/endTime)
are in the same UTC reference frame before comparing start/end/getTime.

In `@src/server/api/routers/bookings.ts`:
- Around line 362-370: There is a TOCTOU race because validateDriverForSlot
(read) and the subsequent ctx.db.insert (write) in the create mutation — and
similarly the update mutation that calls validateDriverForSlot — are separate
operations; wrap the read+write in a single transaction with a stricter
isolation level (e.g., START TRANSACTION; SET TRANSACTION ISOLATION LEVEL
REPEATABLE READ) so validateDriverForSlot runs inside the same transaction
context before calling ctx.db.insert/ctx.db.update, then commit; alternatively
acquire a PostgreSQL advisory lock keyed by driverId or add an exclusion
constraint on the tsrange to prevent overlapping bookings at the DB level and
remove the race. Ensure the fixes are applied to the create handler that calls
validateDriverForSlot and the update handler that calls the same function
(validateDriverForSlot and the corresponding ctx.db.update).
- Around line 127-130: When getTravelTimeMinutes(prevBooking.destinationAddress,
pickupAddress) returns null you silently use FALLBACK_TRAVEL_MINUTES; change
this to log a warning whenever a fallback is used so API failures
(network/quota/invalid address) are visible: after calling getTravelTimeMinutes,
if the result is null, call the existing logger (e.g., logger or processLogger)
to warn that travel-time lookup failed, include the from/to addresses
(prevBooking.destinationAddress and pickupAddress) and that
FALLBACK_TRAVEL_MINUTES is being used, then proceed to compute requiredMinutes =
FALLBACK_TRAVEL_MINUTES + TRAVEL_BUFFER_MINUTES; apply the identical
logging/fallback pattern to the next-booking travel check as well.

---

Duplicate comments:
In `@src/server/api/routers/bookings.ts`:
- Around line 222-235: The duplicate review comment indicates no code change is
needed because listDrivers already conditionally includes email for admins via
the isAdmin check in the protectedProcedure query; resolve this by removing or
marking the duplicate review comment as resolved in the PR (no code edits to
ctx.db/select or the isAdmin conditional in listDrivers are necessary).
- Around line 515-524: Remove the leftover duplicate review marker and clarify
the intent around driver revalidation: delete the stray "[duplicate_comment]"
and, if needed, add a short inline comment near the driverId /
needsDriverValidation logic (variables: driverId, needsDriverValidation,
updatesToApply, existing) explaining that driver revalidation is intentionally
triggered when driverId is present and any of driver/time/address fields change;
keep the boolean condition as-is.

---

Nitpick comments:
In `@src/app/debug/bookings/page.tsx`:
- Around line 365-408: The JSX contains an IIFE that renders the driver
schedule; extract it into a named component (e.g., DriverScheduleTable) to
improve readability and testability: create a new React component
DriverScheduleTable that accepts props bookings (from allBookingsQuery.data),
day (selectedDay) and driverId (form.values.driverId), encapsulate the filtering
(using bookingOverlapsDay), sorting, and rendering logic (including using
formatTimeSlot and styles.bookedSlotsTableBody) inside that component, and
replace the IIFE in page.tsx with <DriverScheduleTable
bookings={allBookingsQuery.data ?? []} day={selectedDay}
driverId={form.values.driverId} />.

In `@src/server/api/routers/bookings.ts`:
- Around line 288-292: The query endpoint that computes drivingTimeMinutes calls
getTravelTimeMinutes (via getEstimatedEndTime) on every request which can
exhaust Google Maps quota; update the .query handler (where drivingTimeMinutes
is computed) to: 1) add a server-side cache keyed by the input pair
(pickupAddress + destinationAddress) with a sensible TTL (e.g., 5–15 minutes)
using your app cache (in-memory or Redis) and return the cached travel time when
present; 2) fall back to FALLBACK_TRAVEL_MINUTES only if both cache miss and API
call fail; and 3) keep the API call guarded with try/catch to avoid throwing on
external API errors. Also ensure the client that calls this tRPC query sets a
non‑zero staleTime (e.g., several minutes) in React Query to avoid refetch
storms.

In `@src/server/db/booking-schema.ts`:
- Around line 43-51: The current composite index bookings_driver_overlap_idx
(driverId, status, startTime, endTime) doesn't support the prev/next lookups
used by validateTravelBetweenAdjacentBookings because the ne(status,
"cancelled") predicate breaks the index prefix; add a supplementary index
optimized for the prev-booking query—e.g., create a new index named
bookings_driver_end_time_idx on (driver_id, end_time) that is either a partial
index with WHERE status <> 'cancelled' or a standalone (driverId, endTime)
index; since Drizzle doesn't expose .where() on indexes yet, implement the
partial index as a raw SQL migration (or add the plain (driverId, endTime)
index) so validateTravelBetweenAdjacentBookings can efficiently find the
previous booking by endTime.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/server/api/routers/bookings.ts`:
- Around line 524-526: The update input schema for the bookings router is
missing the same endTime > startTime validation present in the create schema;
update the Zod schema used by the update resolver (the object containing
startTime: z.string().datetime().optional(), endTime:
z.string().datetime().optional()) to add a .refine() that enforces when both
startTime and endTime are provided that endTime > startTime (same logic as in
the create schema), so that findOverlappingBooking and row integrity are
preserved.
- Around line 319-325: The current logic stores FALLBACK_TRAVEL_MINUTES into
travelTimeCache when getTravelTimeMinutes returns null, which causes silent
serving of the fallback for TRAVEL_TIME_CACHE_TTL_MS; change the behavior in the
block handling getTravelTimeMinutes so that if apiResult is null you set
drivingTimeMinutes = FALLBACK_TRAVEL_MINUTES but do NOT write to travelTimeCache
(skip the cache write) and log a warning/notice similar to
validateTravelBetweenAdjacentBookings to surface the failure; keep cache writes
only when apiResult is a non-null number and continue to use the same cache key
and TTL constants (travelTimeCache, FALLBACK_TRAVEL_MINUTES,
TRAVEL_TIME_CACHE_TTL_MS, getTravelTimeMinutes,
validateTravelBetweenAdjacentBookings).
- Around line 35-47: assertCanAccessBooking currently only allows admins or
agency owners, which blocks assigned drivers; change its signature to accept an
optional bookingDriverId (e.g., assertCanAccessBooking(session, agencyId,
bookingDriverId?)) and treat the caller as allowed if role === "admin" ||
agencyId === session.user.id || bookingDriverId === session.user.id. Then update
getById to fetch the booking first and call assertCanAccessBooking(session,
booking.agencyId, booking.driverId) before returning; leave update and cancel
calls using the original two-argument semantics if drivers should remain unable
to mutate bookings.

---

Duplicate comments:
In `@src/app/debug/bookings/page.tsx`:
- Around line 53-64: The function bookingOverlapsDay builds UTC-day boundaries
using UTC getters from the provided local-midnight Date, which misaligns for
UTC+ zones; update bookingOverlapsDay to use local getters (getFullYear,
getMonth, getDate) when computing dStart/dEnd so the UTC boundaries match how
selectedDay/computedStart are constructed and keep the overlap check (start <
dEnd && end > dStart) unchanged; look for bookingOverlapsDay and ensure it
mirrors the same "local getters -> UTC boundary" approach as computedStart to
fix the off-by-one in UTC+ timezones.

---

Nitpick comments:
In `@src/app/debug/bookings/page.tsx`:
- Around line 240-255: The query for driver availability (availabilityQuery
using api.bookings.isDriverAvailable.useQuery) fires on every change because
staleTime defaults to 0; add a small staleTime (e.g., 1000–2000 ms) to the query
options so edits to pickupAddress/destinationAddress don't immediately refetch
while the user is typing, keeping canCheckAvailability logic unchanged and
preserving caching for calls that may delegate to getTravelTimeMinutes.

In `@src/server/api/routers/bookings.ts`:
- Line 29: travelTimeCache is never removing expired entries which causes
unbounded memory growth; modify the access logic that reads from travelTimeCache
(the Map named travelTimeCache) to prune expired entries when they are
encountered (e.g., on a cache miss or when checking an entry's expiresAt) by
deleting Map keys whose expiresAt <= Date.now(), and optionally perform a small
bounded sweep (e.g., iterate a few keys) on misses to remove other expired items
so the Map stays bounded during long-running processes.

In `@src/server/db/booking-schema.ts`:
- Around line 45-50: The overlap index bookings_driver_overlap_idx on (driverId,
status, startTime, endTime) cannot fully satisfy queries that need a separate
range on endTime (endTime > :bookingStart) because PostgreSQL can only
range-scan one trailing column, so endTime becomes a residual filter; keep the
complementary bookings_driver_end_time_idx on (driverId, endTime) and add a
brief inline comment next to the index definitions (referencing
bookings_driver_overlap_idx and bookings_driver_end_time_idx and
table.driverId/table.status/table.startTime/table.endTime) explaining this
limitation and why the second index is required, or if you prefer to optimize
different query shapes consider adding/ordering an index specifically for the
common query (e.g., (driverId, endTime, startTime)) after profiling.

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/server/api/routers/bookings.ts`:
- Around line 41-53: assertCanAccessBooking currently only allows admins and
agency owners (role "admin" or agencyId === session.user.id) which blocks
assigned drivers from accessing their own bookings; update the guard and its
call sites (getById, update, cancel) so it also permits a session user whose id
matches the booking.driverId. Concretely, change assertCanAccessBooking to
accept an optional driverId (e.g., assertCanAccessBooking(session, agencyId,
driverId?)) or accept the booking object, then in the body allow access when
session.user.id === driverId; update the calls in getById, update, and cancel to
pass the booking's driverId (or booking) before throwing TRPCError.
- Around line 328-336: The code currently caches FALLBACK_TRAVEL_MINUTES when
getTravelTimeMinutes returns null; change the logic so that when apiResult ===
null you set usedFallback = true and drivingTimeMinutes =
FALLBACK_TRAVEL_MINUTES but do NOT write to travelTimeCache (only write cache
entries when apiResult !== null). Locate the block around getTravelTimeMinutes,
cacheKey, travelTimeCache.set and adjust it so travelTimeCache.set is executed
only if apiResult is a real number (apiResult !== null), leaving
usedCached/usedFallback behavior unchanged.
- Around line 539-541: The update input schema for bookings currently defines
startTime and endTime as z.string().datetime().optional() but lacks the same
.refine() check used in the create schema to enforce endTime > startTime, so
supply of both can produce an inverted range and break findOverlappingBooking;
update the "update" input schema (the update Zod object that contains startTime
and endTime) to add a conditional refine that, when both startTime and endTime
are present, parses them and ensures endTime > startTime (mirroring the create
schema's validation) and returns a proper Zod error if the condition fails.

---

Nitpick comments:
In `@src/app/debug/bookings/page.tsx`:
- Around line 447-451: Replace the hardcoded "15 min" in the debug string with a
derived pickup-wait value calculated from the query result: compute pickupWait =
estimatedEndQuery.data.totalBookingMinutes -
estimatedEndQuery.data.drivingTimeMinutes (guard for undefined/null), then
render that pickupWait value (e.g., `${pickupWait} min`) instead of the literal
"15 min"; update the debug lines that reference
estimatedEndQuery.data.totalBookingMinutes and
estimatedEndQuery.data.drivingTimeMinutes to use this derived pickupWait so the
displayed pickup wait always matches the API data.

In `@src/server/api/routers/bookings.ts`:
- Around line 409-412: The current code silences a type mismatch by casting the
transaction object (tx) with "tx as unknown as typeof db" when calling
validateDriverForSlot; instead, update your DbContext type so it accepts both
the normal db and a transaction instance (widen the context type or make it
generic over the DB/transaction shape), then change validateDriverForSlot (and
any other functions that accept DbContext) to use that widened/generic DbContext
type so you can pass tx directly from ctx.db.transaction without any casts;
locate usages around validateDriverForSlot, ctx.db.transaction, and other
callers (the other cast usage referenced in the review) and remove the
double-casts once the DbContext type is adjusted.
- Line 29: The travelTimeCache Map stores entries with expiresAt but never
evicts them, causing unbounded memory growth; modify the module to regularly
purge expired entries by adding a periodic cleanup (e.g., start a setInterval
when the module initializes that iterates travelTimeCache and deletes entries
where entry.expiresAt < Date.now()) or swap the Map for an LRU cache
implementation (e.g., replace travelTimeCache with an instance from the
lru-cache package configured with a max size and TTL) and update any get/set
usages in the functions that reference travelTimeCache to use the new cache API;
ensure the cleanup interval is cleared on shutdown if using setInterval.